Chore/test build#5
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughThis PR introduces build export verification infrastructure. It adds a new test suite to validate the public API exports from the compiled dist/ output, creates supporting test configuration and scripts, and integrates build testing into CI and publish workflows. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
rstest.build.config.ts (1)
8-8: Consider a more specific include pattern.The pattern
test/**/*.test.tswill match all test files in thetest/directory. If this config is specifically for build/dist validation tests, consider using a more targeted pattern liketest/build.test.tsortest/build/**/*.test.tsto avoid accidentally running unrelated tests with this configuration.Suggested change
- include: ['test/**/*.test.ts'], + include: ['test/build.test.ts'],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rstest.build.config.ts` at line 8, The include glob in the config currently uses a broad pattern "test/**/*.test.ts"; narrow it to only build-related tests by replacing that value (the include array entry) with a more specific pattern such as "test/build/**/*.test.ts" or a single file like "test/build.test.ts" so the config only picks up build/dist validation tests; update the include array entry accordingly wherever the include: ['test/**/*.test.ts'] line appears..changeset/good-paws-build.md (1)
1-2: Empty changeset file.This changeset contains no package entries or description. If no version bump is intended (e.g., internal tooling changes), this is fine. Otherwise, add the package and change type:
--- "@rstackio/services": patch --- Add build export verification tests🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.changeset/good-paws-build.md around lines 1 - 2, The changeset file is empty; either add the package entry and change type plus a description or mark it intentionally empty; update the .changeset/good-paws-build.md to include the changeset frontmatter (for example add "@rstackio/services": patch and a short description like "Add build export verification tests") or, if no release is intended, add a brief note in the file indicating this is an intentional no-op changeset.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/build.test.ts`:
- Around line 1-5: Update the top-of-file comment in test/build.test.ts to
reference the correct npm script name: change the referenced command from `pnpm
test:dist` to `pnpm test:build` so the documentation matches package.json; leave
the rest of the smoke test comment unchanged.
- Around line 9-41: Place a TypeScript suppression comment immediately above
each dynamic import of built modules in the test file to avoid type-check errors
during type-checking; specifically add "// `@ts-expect-error`" before the imports
that reference '../dist/safe/index.js', '../dist/data-provider/index.js',
'../dist/mock/index.js', '../dist/logger/index.js', and
'../dist/errors/index.js' so the test assertions (e.g., checks for mod.safe,
mod.createSafeProvider, mod.isMockEnabled, mod.Logger, and the import resolution
check) still run but the TS compiler won’t fail because the dist files don’t
exist at type-check time.
In `@tsconfig.json`:
- Line 15: The tsconfig's "include": ["src", "test"] causes type errors because
test/build.test.ts imports compiled files (../dist/*/index.js) that don't exist
at type-check time; fix by excluding that test file from the main TS project and
adding a dedicated config for build tests: remove "test" (or add an "exclude":
["test/build.test.ts"]) from the primary tsconfig.json and create a separate
tsconfig.build-test.json that includes the test directory (or specifically
test/build.test.ts) for runtime-only checks, or alternatively add targeted //
`@ts-ignore` or // `@ts-expect-error` comments on the dynamic import lines in
test/build.test.ts if you prefer the inline suppression approach.
---
Nitpick comments:
In @.changeset/good-paws-build.md:
- Around line 1-2: The changeset file is empty; either add the package entry and
change type plus a description or mark it intentionally empty; update the
.changeset/good-paws-build.md to include the changeset frontmatter (for example
add "@rstackio/services": patch and a short description like "Add build export
verification tests") or, if no release is intended, add a brief note in the file
indicating this is an intentional no-op changeset.
In `@rstest.build.config.ts`:
- Line 8: The include glob in the config currently uses a broad pattern
"test/**/*.test.ts"; narrow it to only build-related tests by replacing that
value (the include array entry) with a more specific pattern such as
"test/build/**/*.test.ts" or a single file like "test/build.test.ts" so the
config only picks up build/dist validation tests; update the include array entry
accordingly wherever the include: ['test/**/*.test.ts'] line appears.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.changeset/good-paws-build.md.github/workflows/ci.yml.github/workflows/publish.ymlpackage.jsonrstest.build.config.tstest/build.test.tstsconfig.json
Summary by CodeRabbit